r+, though could probably use a handful of tests to guard against the ever-present "whoops, got my boundary conditions slightly wrong" bugs. I can write those if you don't feel like it though.

Thanks!

Yeah, I wasn't sure where they should go (i.e. at the bottom of the file or in the tests directory), are there any rules/conventions about this?

(Also, I'm away for a week or so, and won't be able to do anything, so feel free to add the tests.)

Graydon Hoare [email protected] wrote:

r+, though could probably use a handful of tests to guard against the
ever-present "whoops, got my boundary conditions slightly wrong" bugs.
I can write those if you don't feel like it though.

Thanks!

---

Reply to this email directly or view it on GitHub:
#4313 (comment)

@dbaupp library tests go at the bottom of the file, language tests go in the tests directory.

@graydon Added some tests, and found that I had met the "'negative' unsigned numbers act strangely" bug.

Previously, calling for uint::range_step(10, 0, -1) |_| { .. } didn't iterate at all (the expected behaviour is iterating over 10,9,...,1), because -1 > 0 for unsigned numbers and so it didn't detect that it should be iterating downwards. Fixed by breaking out the single range_step function into 2 functions iterating in a specific direction.

I'm slightly uncomfortable about having a different API for signed vs unsigned (i.e. range_step vs range_step_up & range_step_down), but I'm not sure there is a better resolution.

@dbaupp Thanks.

@graydon What do you think of these API changes? Is it ok to have two functions here? We could also just use int for all the step values.

Yeah, I'd just take an int for the step and have one variant.

I'm unsure if it should be int (or i64?) or the equivalently sized signed type (i.e. i8 for u8). The former leads to either inexpressible increments for u64 (or inefficiencies, for i64) on 32-bit platforms (is this a concern?) and the latter ends up cluttering up the inst modules with a T_SIGNED type for just this purpose.

(Unless there is already a technique to get the type iN from uN already?)

A whole new function for range_rev feels unnecessary. Can't range just count down rather than up when its first argument is greater than its second? i.e. have it count over the range [first..second) rather than [lo..hi).

I vote for adding T_SIGNED and making the step value a signed type of the equivalent size. I'm not sure about range_rev. #1817 suggested to make range also iterate backwards but notes that python's range function doesn't automatically count backwards without a step of -1.

Considering Python's precedent, I vote for the following functions:

int::range(lo: int, high: int);
int::range_step(start: int, end: int, step: int);
uint::range(lo: uint, high: uint);
uint::range_step(start: uint, end: uint, step: int);

Where reverse is just achieved by providing a negative step value to range_step, and the final element is always omitted.

I'm adding the T_SIGNED type and making the simplification @brson suggested. Waiting for tests to run.

Yeah. +1, these are convenience functions to be used casually; step will almost always be 1, 2, -1, -2 or a similarly small number. Iterating by MAXINT-ish steps is rare and probably best done manually or by a user-written loop / iterator.

Merged at last! 982cf90

(Oh, and thanks to @dbaupp :-)